-
Notifications
You must be signed in to change notification settings - Fork 288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add Orders to Account Profile #507
Conversation
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
…ent Library updates Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
…gs (orders) Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
…cross app Signed-off-by: Erik Kieckhafer <ek@ato.la>
…commerce/reaction-next-starterkit into feat-kieckhafer-newAccountPage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nnnnat Yes, that is expected if you haven't added the translations data. Instructions are here: reactioncommerce/reaction#4981 Let me know if that fixes the issue. "new" is probably also wrong, it's probably not capitalized in your version. If you add them to the settings it'll change. |
@kieckhafer that makes sense I'll retest in a bit |
@kieckhafer Regarding the copy issue Nat mentioned, it's still possible that people could eventually have long status labels when we allow customizing them. Can we do something with CSS, maybe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good. Just a couple small suggestions.
@@ -48,6 +48,7 @@ | |||
"compression": "^1.7.3", | |||
"cookie-parser": "^1.4.3", | |||
"cookie-session": "^2.0.0-beta.3", | |||
"date-fns": "^1.30.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a decision to use date-fns
instead of moment for new projects? It's fine with me but I'd like to see the decision recorded and communicated if it hasn't been.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a chat in Slack about a light(er)weight solution for dates, considering we need very little. Made a DR and tagged you in Notion.
} | ||
|
||
onTrackShipmentButtonClick() { | ||
// TODO: What do we do to track a shipment? Link to a specific provider website? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'll add trackingUrl
prop on fulfillmentGroup
in my tracking PR, and then you can render that in a link here? Seems better than recording the carrier and trying to dynamically build links to different carriers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that sounds like a good idea.
Signed-off-by: Erik Kieckhafer <ek@ato.la>
@aldeed updated based on comments, ready for a look
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Expands upon #499 to add Orders data to profile page
Impact: minor
Type: feature
Issue
Add Orders data to Account Profile
Solution
profileAddressBook
andprofileOrders
pages. This is necessary due to the way we do pagination, our existing method broke when usingasPath
to display different components (Orders
orAddressBook
) on a single page based on the URL.5
and10
page size to our default pagination. Since we use the same component for CatalogItems / Tags, and now orders, and these return various dimensions of results we should allow more than just 20,60, or 100 for the results, as 20 is far too manyOrderCard
's to scroll throughThank You
page after an order is placed to useOrderCard
, with theHeader
portion expanded by default (it's not expanded by default on the profile page).Breaking changes
None
Testing
/profile/orders
as a non-logged in user, and see a 404 / page not found/profile/orders
as a logged in user, see that orders show up and match our designs: https://zpl.io/V0ypYWR